[CXP-204] improve rate limit and temporary error handling by johnallers · Pull Request #121 · ConductorOne/baton-github

johnallers · 2026-02-04T12:34:53Z

Summary

Fix rate limit errors being misclassified as PermissionDenied instead of Unavailable, causing syncs to fail instead of retry
Add proper handling for *github.RateLimitError and *github.AbuseRateLimitError error types using errors.As()
Extract rate limit data (reset time, limit, remaining) and attach to gRPC status details for proper backoff
Add handling for 503/502/504 temporary server errors as Unavailable (retryable)

Problem

When go-github blocks requests client-side due to rate limits, it returns a RateLimitError with a synthetic 403 response that has empty headers. The existing isRatelimited() check failed because it expects X-Ratelimit-Remaining header to equal "0", but the synthetic response has no headers. This caused rate limit errors to be misclassified as PermissionDenied, which is not retried by the SDK.

Solution

Check for go-github error types before checking HTTP status codes:

*github.RateLimitError → extract Rate.Reset, Rate.Limit, Rate.Remaining → codes.Unavailable with rate limit details
*github.AbuseRateLimitError → use RetryAfter duration → codes.Unavailable with rate limit details
503/502/504 responses → codes.Unavailable (retryable)

The SDK's retry logic now receives proper rate limit data to calculate appropriate backoff times.

Test plan

Build passes
Tests pass
Lint passes
Test with a GitHub org that hits rate limits in service mode

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Improved detection and handling of rate-limit errors, including clearer retry timing and guidance for both standard and abuse-triggered limits.
- Rate-limit details are now attached to error responses, providing more actionable retry information.
- Temporary server outages (502/503/504) are more reliably detected and surfaced as "service unavailable" for affected requests.

linear · 2026-02-04T12:34:56Z

CXP-204 Improve baton-github connector rate limit handling

coderabbitai · 2026-02-04T12:35:16Z

Walkthrough

Adds helpers to detect GitHub rate/abuse limits and temporary server errors, build RateLimitDescription from github.Rate or Retry-After durations, and attach that detail to gRPC status errors; updates wrapGitHubError to propagate these details and treat 502/503/504 as temporarily unavailable.

Changes

Cohort / File(s)	Summary
Rate-limit & error-wrapping helpers `pkg/connector/helpers.go`	Added `rateLimitDescriptionFromRate`, `rateLimitDescriptionFromRetryAfter`, `wrapErrorWithRateLimitDetails`, `isTemporarilyUnavailable`; enhanced `wrapGitHubError` to handle `RateLimitError` and `AbuseRateLimitError`, preserve existing rate-limit checks, and attach `RateLimitDescription` to returned gRPC errors. Also added required imports (`errors`, `time`, `status`).

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Connector
  participant GitHubAPI
  participant gRPCStatus

  Client->>Connector: Request
  Connector->>GitHubAPI: Call GitHub
  alt RateLimitError (with Rate)
    GitHubAPI-->>Connector: RateLimitError (with Rate)
    Connector->>Connector: rateLimitDescriptionFromRate()
    Connector->>gRPCStatus: wrapErrorWithRateLimitDetails()
    gRPCStatus-->>Connector: gRPC error with RateLimitDescription
    Connector-->>Client: rate-limited gRPC error
  else AbuseRateLimitError (Retry-After)
    GitHubAPI-->>Connector: AbuseRateLimitError (Retry-After)
    Connector->>Connector: rateLimitDescriptionFromRetryAfter()
    Connector->>gRPCStatus: wrapErrorWithRateLimitDetails()
    gRPCStatus-->>Connector: gRPC error with RateLimitDescription
    Connector-->>Client: rate-limited gRPC error
  else TemporaryServerError (502/503/504)
    GitHubAPI-->>Connector: 502/503/504
    Connector->>Connector: isTemporarilyUnavailable()
    Connector-->>Client: temporary-unavailable error
  else OtherError
    GitHubAPI-->>Connector: Other error
    Connector-->>Client: wrapped error (existing behavior)
  end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 I nibble at headers, timers, and rate,
I craft retry-after crumbs to annotate,
I tuck a detail in gRPC's nest,
So callers know when to pause and rest,
A rabbit marks the API gate. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: improving rate limit and temporary error handling in the connector helpers, which matches the file modifications and PR objectives.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch jallers/cxp-204

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

When go-github blocks requests client-side due to rate limits, it returns a RateLimitError with a synthetic 403 response that has empty headers. The existing isRatelimited() check failed because it expects the X-Ratelimit-Remaining header to equal "0", but the synthetic response has no headers at all. This caused rate limit errors to be misclassified as PermissionDenied, which is not retried by the SDK. Changes: - Check for *github.RateLimitError and *github.AbuseRateLimitError types using errors.As() before checking HTTP status codes - Extract rate limit data (reset time, limit, remaining) from the error and attach it to the gRPC status details for proper backoff - Add isTemporarilyUnavailable() to handle 503, 502, and 504 errors - Return codes.Unavailable for all these cases, enabling SDK retry logic Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

pkg/connector/helpers.go

Prevent creating misleading timestamps when rate.Reset.Time is zero, matching the defensive pattern used in other rate limit functions. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@pkg/connector/helpers.go`:
- Line 202: The lint error comes from using the redundant embedded field
selector Time when calling IsZero on a github.Timestamp; change the check in the
rate handling code from using rate.Reset.Time.IsZero() to rate.Reset.IsZero()
(i.e., call IsZero directly on rate.Reset which embeds time.Time) so the lint
warning is resolved; update any other occurrences in the same file/function that
reference rate.Reset.Time to use rate.Reset instead.

pkg/connector/helpers.go

johnallers requested a review from a team February 4, 2026 12:34

johnallers force-pushed the jallers/cxp-204 branch from 7896b65 to 1498c3e Compare February 4, 2026 18:56

johnallers requested a review from a team February 4, 2026 19:55

johnallers changed the title ~~fix: improve rate limit and temporary error handling~~ [CXP-204] improve rate limit and temporary error handling Feb 4, 2026

luisina-santos approved these changes Feb 4, 2026

View reviewed changes

johnallers self-assigned this Feb 4, 2026

johnallers force-pushed the jallers/cxp-204 branch from 1498c3e to b125940 Compare February 5, 2026 11:25

johnallers assigned btipling and unassigned johnallers Feb 5, 2026

FeliLucero1 reviewed Feb 5, 2026

View reviewed changes

pkg/connector/helpers.go Show resolved Hide resolved

pkg/connector/helpers.go Outdated Show resolved Hide resolved

fix: add zero-time check in rateLimitDescriptionFromRate

927707f

Prevent creating misleading timestamps when rate.Reset.Time is zero, matching the defensive pattern used in other rate limit functions. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

coderabbitai bot reviewed Feb 5, 2026

View reviewed changes

pkg/connector/helpers.go Outdated Show resolved Hide resolved

remove embedded field

aa1a7d9

FeliLucero1 approved these changes Feb 5, 2026

View reviewed changes

btipling approved these changes Feb 5, 2026

View reviewed changes

johnallers merged commit 89fc626 into main Feb 5, 2026
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CXP-204] improve rate limit and temporary error handling#121

[CXP-204] improve rate limit and temporary error handling#121
johnallers merged 3 commits intomainfrom
jallers/cxp-204

johnallers commented Feb 4, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

linear bot commented Feb 4, 2026

Uh oh!

coderabbitai bot commented Feb 4, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

johnallers commented Feb 4, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Solution

Test plan

Summary by CodeRabbit

Uh oh!

linear bot commented Feb 4, 2026

Uh oh!

coderabbitai bot commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

johnallers commented Feb 4, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Feb 4, 2026 •

edited

Loading